Skip to content

New Project > Select Repository #4443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 23, 2021
Merged

New Project > Select Repository #4443

merged 1 commit into from
Jun 23, 2021

Conversation

AlexTugarev
Copy link
Member

@AlexTugarev AlexTugarev commented Jun 9, 2021

This PR introduces the project creation flow (gitpod.io/new)


GH App Change

⚠️ Please note, this change requires a significant GH App config change to be aligned with any deployment: instead of redirecting to an URL of the dashboard, it should redirect to the /api/apps/github/setup route introduced here.

Screen Shot 2021-06-14 at 05 39 29


Preview environment

The URL is https://at-select-repo.staging.gitpod-dev.com/new

⚠️ Please note, this preview environment requires an additional re-deployment with werft from within a Gitpod workspace using werft run github -s chart/values.yaml -f, where the provided values should contain the GH App config. In addition, a secret with the private key of the GH App should be created like this kubectl create secret generic server-github-app-cert ...


New Project

The drop-down list contains the user's account by default. If the GH App is installed on Orgs of the users and the user granted read:org permissions, these accounts should be visible as well.

Screen Shot 2021-06-14 at 05 52 26

Screen Shot 2021-06-14 at 05 29 35

  • This list should be filtered to exclude repositories already in used in any project.

Select Git Provider

Screen Shot 2021-06-14 at 05 29 41

Reconfigure GH App installations

You will see the GH App configuration in a pop-up window. For the preview environment this is a test app of mine.

Screen Shot 2021-06-14 at 05 30 05

Select a team

Listing of all teams of a user.

  • Incorporate "New Team" inline

Screen Shot 2021-06-14 at 05 30 18

List projects of a team

This is a starting point for the Projects overview.

Screen Shot 2021-06-14 at 05 30 52

Closes #4352

@AlexTugarev AlexTugarev force-pushed the at/select-repo branch 10 times, most recently from 4f6d80b to 0d15bc6 Compare June 13, 2021 11:20

//#region Projects concerns
//
async getProviderRepositoriesForUser(params: { provider: string }): Promise<ProviderRepository[]> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is implemented in EE space because the github-app.ts is also there.

@svenefftinge, it's ok to move the github-app.ts to the non-EE sources?

@AlexTugarev AlexTugarev changed the title WIP New Project > Select Repository New Project > Select Repository Jun 14, 2021
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see this coming to life @AlexTugarev! Left some minor UX comments below. 🏀

</div>
<div className="p-3 bg-gray-100">
<div className="text-gray-500 text-center">
Repository not found? <a href="javascript:void(0)" onClick={e => reconfigure()} className="text-gray-400 underline underline-thickness-thin underline-offset-small hover:text-gray-600">Reconfigure</a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Is it possible to switch to the org where the app has been installed after installing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

When selecting Reconfigure and editing the installations of the App on GH, you may have either:

  1. Install on new Account/Org
  2. Selected additional Repos (or selected all)
  3. Deleted the installation from an Account/Org

All these kinds of changes needs to be reflected in the action (Reconfigure) and also updated after the action is finished, which I doubt happens in all cases. At least after deletion, we're not getting redirected :-( So in that case, where the user does (need to) close the GH pop-up window, we need to update!

</div>
<div className="w-4/12 flex justify-end">
<div className="flex self-center hover:bg-gray-200 dark:hover:bg-gray-700 rounded-md cursor-pointer opacity-0 group-hover:opacity-100">
<button className="primary py-1" onClick={() => selectRepo(r)}>Select</button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: If we don't use any other label for this button we could remove it and use the whole div as a hyperlink as done with similar modals (see new workspace, parallel workspaces, etc).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would that apply to the Select Team page as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case the project is already part of a team we could trigger a modal with a CTA button to request access to the team, instead of having conditional buttons here Add Project and Select Team. WDYT?

@AlexTugarev AlexTugarev force-pushed the at/select-repo branch 2 times, most recently from 45d6156 to 9e6aaab Compare June 14, 2021 14:09
@AlexTugarev
Copy link
Member Author

#4443 (comment)

In case the project is already part of a team we could trigger a modal with a CTA button to request access to the team, instead of having conditional buttons here Add Project and Select Team. WDYT?

@gtsiolis, thanks for bringing this up! It's a rather conceptual question, so let's discuss.

If a repository (aka project) is already linked to a team, what is the implication to the overall concept and the creation flow?Potentially incomplete list of options:

  1. Exclude taken repositories on Create Project page.
  2. Disable taken repositories on Create Project page.
  3. Allow every accessible repo on Create Project page, then add an invitation method.

@svenefftinge
Copy link
Member

svenefftinge commented Jun 15, 2021

/werft run

👍 started the job as gitpod-build-at-select-repo.40

@AlexTugarev AlexTugarev force-pushed the at/select-repo branch 3 times, most recently from 9366cd1 to 1bb1af2 Compare June 15, 2021 09:53
@AlexTugarev
Copy link
Member Author

The check for the state parameter on setup callbacks provides a nice migration path. When installing the app from GH, we get redirected to /install-github-app like before

Screen Shot 2021-06-15 at 14 13 11

Screen Shot 2021-06-15 at 14 13 22

@svenefftinge
Copy link
Member

svenefftinge commented Jun 16, 2021

  • When I add an org, it replaces the org I previously added.
  • Even when I already have a project for an org, I don't see that org anymore in the list.
  • When I land on new, nothing in the combo box is selected.
  • When I added an app installation, the newly added installation is not selected
  • When I have added the 'orgs' scope, I still don't see the orgs for which installations exist

@svenefftinge
Copy link
Member

svenefftinge commented Jun 16, 2021

  • The new project flow should not have any menus or so. See design docs. But we'll need to allow users to abort the flow (cc @gtsiolis )

@AlexTugarev
Copy link
Member Author

When I have added the 'orgs' scope, I still don't see the orgs for which installations exist

@svenefftinge, that is true as long as the gitpod-dev OAuth app isn't approved for the org. On https://at-select-repo.staging.gitpod-dev.com/integrations you can select Manage github.com or directly navigate to
https://github.com/settings/connections/applications/51329bab66290acdf39f and make sure you can approve an organization to test with.

@AlexTugarev AlexTugarev force-pushed the at/select-repo branch 2 times, most recently from 4c50602 to a61e38a Compare June 18, 2021 08:32
@AlexTugarev
Copy link
Member Author

@svenefftinge, I updated the PR with following changes:

  • When I add an org, it replaces the org I previously added.

I'm not sure, what that means. The Reconfigure action (showing GitHub's installation page) combines several options: installing, updating, or deleting.

  • Even when I already have a project for an org, I don't see that org anymore in the list.

That should be solved now. knownOrgs are preserved in User.additionalData, so that when you come back later to /new, you should be able to browse the repos of that org.

  • When I land on new, nothing in the combo box is selected.

That might be just related to the missing org, I suppose. In general it takes up to seconds if you have many orgs accessible to the OAuth App of the Gitpod installation.

  • When I added an app installation, the newly added installation is not selected

Done. They should be selected now properly. Also adding updateAt for installations and repositories. Repositories are sorted now.

  • When I have added the 'orgs' scope, I still don't see the orgs for which installations exist

This is fixed in the sense that known orgs are preserved on backend now.

@AlexTugarev
Copy link
Member Author

The new project flow should not have any menus or so. See design docs.

@svenefftinge and @gtsiolis, I aligned with /new-teams on that.

I'm happy to reduce the menus. For both?

@AlexTugarev AlexTugarev marked this pull request as ready for review June 18, 2021 14:04
@AlexTugarev AlexTugarev requested a review from jankeromnes June 21, 2021 13:16
Copy link
Member

@svenefftinge svenefftinge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well. It needs additional passes, of course, but since it is not activated / visible by default it would be good to merge ASAP. So we can iterate in smaller batches.

@@ -11,7 +11,7 @@ export class TeamsAndProjects1622468446118 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<any> {
await queryRunner.query("CREATE TABLE IF NOT EXISTS `d_b_team` (`id` char(36) NOT NULL, `name` varchar(255) NOT NULL, `slug` varchar(255) NOT NULL, `creationTime` varchar(255) NOT NULL, `deleted` tinyint(4) NOT NULL DEFAULT '0', `_lastModified` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), PRIMARY KEY (`id`), KEY `ind_dbsync` (`_lastModified`)) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;");
await queryRunner.query("CREATE TABLE IF NOT EXISTS `d_b_team_membership` (`id` char(36) NOT NULL, `teamId` char(36) NOT NULL, `userId` char(36) NOT NULL, `role` varchar(255) NOT NULL, `creationTime` varchar(255) NOT NULL, `deleted` tinyint(4) NOT NULL DEFAULT '0', `_lastModified` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), PRIMARY KEY (`id`), KEY `ind_teamId` (`teamId`), KEY `ind_userId` (`userId`), KEY `ind_dbsync` (`_lastModified`)) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;");
await queryRunner.query("CREATE TABLE IF NOT EXISTS `d_b_project` (`id` char(36) NOT NULL, `cloneUrl` varchar(255) NOT NULL, `teamId` char(36) NOT NULL, `appInstallationId` varchar(255) NOT NULL, `creationTime` varchar(255) NOT NULL, `deleted` tinyint(4) NOT NULL DEFAULT '0', `_lastModified` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), PRIMARY KEY (`id`), KEY `ind_teamId` (`teamId`), KEY `ind_dbsync` (`_lastModified`)) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;");
await queryRunner.query("CREATE TABLE IF NOT EXISTS `d_b_project` (`id` char(36) NOT NULL, `name` varchar(255) NOT NULL, `cloneUrl` varchar(255) NOT NULL, `teamId` char(36) NOT NULL, `appInstallationId` varchar(255) NOT NULL, `creationTime` varchar(255) NOT NULL, `deleted` tinyint(4) NOT NULL DEFAULT '0', `_lastModified` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), PRIMARY KEY (`id`), KEY `ind_teamId` (`teamId`), KEY `ind_dbsync` (`_lastModified`)) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you plan to manually update the staging DB?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point! this is already on staging.
yes, it would be best to do a manual update.

@@ -1205,6 +1208,27 @@ export interface Project {
deleted?: boolean;
}

export interface ProjectInfo {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why copying the whole Project shape instead of referencing it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just learned that DBProject isn't implementing Project at all.
So, yeah it doesn't make sense to duplicate.

Besides that, we introduced xInfo shapes to augment entities, to make it works nicer with the frontend.

//#region Projects concerns
//
async getProviderRepositoriesForUser(params: { provider: string, hints?: object }): Promise<ProviderRepository[]> {
const user = this.checkAndBlockUser("getProviderRepositoriesForUser");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method should go the respective GitProvider impl. We should not add dependencies to GitHub (octokit) across our code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for moving out. I'm thinking of a GitHubAppSupport injectable.

FYI we have two octokit dependencies

  • first in our GitHub support (auth provider, context parsers, etc.)
  • second comes in via probot
    Both parts live in different contexts. More specifically: the GitHub support is instantiated per Git Provider host.

BTW it's impossible impractical to sync both octokit versions both. Also, obtaining GH App's authenticated API (octokit) is done via GitHubApp.server.probotApp.auth().


const project: Project = {
id: uuidv4(),
name,
Copy link
Contributor

@jankeromnes jankeromnes Jun 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Since name is also used as a slug, maybe validate it here against a regex? For example like so (maybe without spaces):

if (!/^[A-Za-z0-9 _-]+$/.test(name)) {
throw new Error('Please choose a team name containing only letters, numbers, -, _, or spaces.');
}

@AlexTugarev AlexTugarev force-pushed the at/select-repo branch 2 times, most recently from e961ad6 to 0fa4916 Compare June 23, 2021 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install GitHub App
4 participants